🤔 this would be a breaking change (which means losing 30% of users, see #1065)
I don't think the safari/webkit version is important enough for that, I never gated features on it.

thanks for the review!

I never gated features on it

maybe I'm misunderstanding what you mean by that, but there's https://github.com/webrtcHacks/adapter/blob/main/src/js/common_shim.js#L356 and #1082 which details an uncaught (by adapter) bug related to the version parsing for Safari 12.

@fippo was not including this in 9.0.0 an oversight or are you not planning to merge this altogether?

I think the main concern here is still that even with a breaking change this would folks to update their code... which will reduce the adoption rate (not great already). #1160 is the next breaking change so this might ride along. But the alternative would be to expose the Safari version with a different name and update the adapter code?

BTW @fippo, I think nobody uses the adapter's Safari version, since it is useless being always 605 (I parse it myself for Safari while for other browsers I lean on adapter). Thus, this is not so breaking change as it looks.

I know it is useless but changing such behavior is a breaking change in semver for better or worse.

I think the main concern here is still that even with a breaking change this would folks to update their code...

I don't see how that's true if the current behaviour is just broken/useless as you mention. No one using a adapter today can rely on the Safari version parsing, so I don't see why they would have to change their code 🤔

But the alternative would be to expose the Safari version with a different name and update the adapter code?

Totally your call, I think everything is better than it not being useful at all, but exposing a custom property for the safari version (not sure if you had something else in mind) sounds less obvious to me than fixing the version parsing

Sorry for taking so long on weighing in here. I'd like to avoid feature gating based on user agent parsing altogether, and I think adding the proper Safari version now without a clear benefit to adapter is a breaking change I'd like to avoid.

Not saying we won't need this in the future, but until then I'd like us to keep it as stable as possible given the intention of this project. The bug you mentioned is valid, but also has a clear workaround. If anything that bug should be fixed by feature testing and fallback rather than user agent parsing to begin with, but I might be able to be swayed on that.

In any case, I love the contribution, and think we should leave it open. As @fippo mentioned we might include this with the next breaking change, or as part of fixing the above-mentioned bug.

yeah, definitely agree that this should only be included in a major version update!

An initial intention was to use adapter's browser parsing which is mentioned in the project's README:

You might want to use adapters browser detection which detects which webrtc quirks are required. ... for webrtc engine detection (which will for example detect Opera or the Chromium based Edge as 'chrome') and adapter.browserDetails.version for the version according to the user-agent string.

and that's not very useful for Safari as is.

Yeah I think we need to remove that from the README, from when this project was created to now, a lot has happened to browser detection, user agent strings etc. to the point where it's much better to use a library dedicated to this, rather than relying on adapter.

Still we need to support the legacy for a while longer, and probably do a major release at some point that will remove exposing these fields and provide a good alternative route for people that rely on this today.

@dagingaa, it seems you concentrate on a global view, while I think this should be treated just as a fix for #1082. When you fix the issue reported almost 4 years ago, you may then wonder about possible refactoring. First and foremost, the lib should just work, not be politically correct (according to not-use-the-user-agent-string orthodoxes).

not be politically correct (according to not-use-the-user-agent-string orthodoxes).

The user agent string is not a reliable method to get information anymore, and thus should not be used as such. Further, relying on adapter for this is backwards and will actually just hurt your application rather than help it when there are good alternatives.

Internally, adapter needs to use any means necessary to add workarounds, such as checking the user agent string. We do that today, and will continue to do that tomorrow. However, exposing that to end users is a mistake I personally think we should stop doing. Changing the detection code everywhere means additional risk which I think is unnecessary.

it seems you concentrate on a global view, while I think this should be treated just as a fix for #1082.

This PR is changing the global view, thus we need to care about it (for the aforementioned reasons). Fixing #1082 in isolation should be a much simpler fix, which we very much welcome, but introducing a major release breaking change is a huge ask which does need to be taken into very careful consideration.

In the meantime we are lucky there exists a workaround.

#1082 is an issue that was fixed in libWebRTC back in late 2018. Safari has been slow updating it but that means 2019 in Safari 13 still. The issue was marginal in 2021 already, in 2025 I doubt it is relevant? (and if not my condolences to the users on that browser)

Using adapter for UA detection was convenient in the early days (see the 2015 version here), in particular since one wanted the "webrtc version" which was the same in Chrome and Opera. I haven't seen much use of that in recent years (and even the samples use it sparingly). For the most part one can write decent feature detection (please do not create a peerconnection for that... well, if you do in Safari I don't mind)

Is relying on adapters version something you should do in 20232025? Probably not👎
If version detection is broken for Safari and that discourages usage of adapter for this purpose then 👍
Do it yourself (like here) 👍

#1161 is now part of v9.0.2